Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ConfigRawParams: When setting a parameter changes the number of parameters, do a full refresh. and check if reboot is needed #3271

Merged
merged 3 commits into from
Mar 4, 2024

Conversation

EosBandi
Copy link
Collaborator

@EosBandi EosBandi commented Jan 6, 2024

If a param change causing the change in the number of parameters, the parameter subsystem could stuck in a "downloading is in progress" state. This pr:
removes the name check, since not only _enable parameters can change the number of parameters
Add a check at the end of Write Params and if the reported param number not equals with the current number, it notify the user and do a param refresh.
This fixes #3198

@EosBandi EosBandi changed the title ConfigRawParams: When setting a parameter changes the number of parameters, do a full refresh. ConfigRawParams: When setting a parameter changes the number of parameters, do a full refresh. and check if reboot is neded Jan 6, 2024
@Hwurzburg
Copy link
Contributor

THANKS! I see the build artifact but not sure how to run it locally for some testing....its an aab file?

@CraigElder CraigElder changed the title ConfigRawParams: When setting a parameter changes the number of parameters, do a full refresh. and check if reboot is neded ConfigRawParams: When setting a parameter changes the number of parameters, do a full refresh. and check if reboot is needed Jan 8, 2024
@davidbuzz
Copy link
Contributor

davidbuzz commented Jan 8, 2024

param refresh dangerous when armed, as low bandwidth or long range links can be adversely affected - i support it if its tweaked to only do this when disarmed.

@rmackay9
Copy link
Contributor

rmackay9 commented Jan 8, 2024

PeterH suggested that we should only do the full refresh when the vehicle is disarmed.

@Hwurzburg
Copy link
Contributor

Hwurzburg commented Jan 14, 2024

while testing I got this error:
image
may or may not be related

Otherwise seems to work well....I agree with Pete's comment re refresh only while disarmed...

this PR is really needed!!! thanks

@EosBandi
Copy link
Collaborator Author

while testing I got this error:
Based on where it happened in the code, this is unrelated. Do you have steps to repro ?

Otherwise seems to work well....I agree with Pete's comment re refresh only while disarmed...

Indeed, added check for armed state.

@Hwurzburg
Copy link
Contributor

@EosBandi will try to reproduce...I was just randomly enablilng/changing parameters that I know need reboots

@meee1 meee1 merged commit cd16a4c into ArduPilot:master Mar 4, 2024
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Param loading Green line does not go away
8 participants